[WIP]: Do not store tag in uninhabited enum variants, or in the single inhabited variant. #145337
+954
−161
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
rustbot
added
A-LLVM
This comment has been minimized.
This comment has been minimized.
zachs18
force-pushed
the
enum-uninhabited-or-single-variant-no-tag
branch
from
7095fd5 to
76ded17
Compare
This comment has been minimized.
This comment has been minimized.
3 tasks
This comment has been minimized.
This comment has been minimized.
…s enum representations
…rate after the commits which add the relevant layout optimizations.
…ommits which add the relevant layout optimizations.
…in a better layout)
zachs18
force-pushed
the
enum-uninhabited-or-single-variant-no-tag
branch
from
e1f38f7 to
6fbb6b7
Compare
rustbot
added
the
T-rust-analyzer
Zalathar
added a commit
to Zalathar/rust
that referenced
this pull request
Aug 29, 2025
Remove TmpLayout in layout_of_enum 09a3846 from <rust-lang#103693> made LayoutData be owned instead of interned in `Variants::Multiple::variants`[^1], so there's no need for `TmpLayout` in layout_of_enum anymore, and we can just store the variants' layouts directly in the prospective `LayoutData`s' `variants` fields. This should have no effect on semantics or layout. (written as part of rust-lang#145337 but not related to the layout optimizations in that PR) [^1]: see line 1154 of `compiler/rustc_target/src/abi/mod.rs` in the linked commit; `Variants::Multiple::variants` effectively changed from `IndexVec<.., Layout<'tcx>>` to `IndexVec<.., LayoutData>` where the `LayoutData`s are not interned as `Layout`s (`LayoutData` was at the time called `LayoutS`)
rust-timer
added a commit
that referenced
this pull request
Aug 29, 2025
Rollup merge of #145387 - zachs18:remove-tmplayout, r=cjgillot Remove TmpLayout in layout_of_enum 09a3846 from <#103693> made LayoutData be owned instead of interned in `Variants::Multiple::variants`[^1], so there's no need for `TmpLayout` in layout_of_enum anymore, and we can just store the variants' layouts directly in the prospective `LayoutData`s' `variants` fields. This should have no effect on semantics or layout. (written as part of #145337 but not related to the layout optimizations in that PR) [^1]: see line 1154 of `compiler/rustc_target/src/abi/mod.rs` in the linked commit; `Variants::Multiple::variants` effectively changed from `IndexVec<.., Layout<'tcx>>` to `IndexVec<.., LayoutData>` where the `LayoutData`s are not interned as `Layout`s (`LayoutData` was at the time called `LayoutS`)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR contains 3 or 4 changes which could probably be split up:
First (first commit): split out into #145387
Second (second commit) (needed for the actual layout changes to be sound with
offset_of!on uninhabited variants' fields): Keep track of uninhabited non-abset variants' layouts, even if we use an untagged (Variants::Single) or uninhabited (Variants::Empty) layout. This is necessary as otherwiseoffset_of!will "conjure" a layout for uninhabited variants where all fields are at offset 0, which conflicts with partial initialization1.Third (sixth commit): Do not make space for the enum tag in uninhabited variants in the tagged enum layout calculation. This optimizes enums where the uninhabited variants are the largest, and reduces their size by up to the size of the tag. e.g.
enum Foo { A, B(i32, !) }was 8 bytesA: (tag, uninit), B: (tag, i32, !)and is now 4 bytesA: (tag,), B: (i32, !). Additionally, enums with only zero-sized uninhabited variants are special-cased to return an uninhabited layout of the correct size and alignment without a tag at all, as otherwise we get weird edge cases2.Fourth (seventh commit): Add an additional enum layout that does not encode a tag. This is similar to the existing "single-present-variant" optimization, but works even if some of the uninhabited variants are "present" (have non-1-ZST fields). This works by calculating the layout of the inhabited variant as a struct, and padding it to the maximum size and alignment necessary to fit each uninhabited variant.
The layout changes probably need an MCP and I haven't squashed fully, so draft for now.
r? ghost
Footnotes
This isn't an issue before this PR because the layout algorithm only produces
Variants::EmptyorVariants::Singlewhen all uninhabited variants are "absent", i.e. have only 1-ZST fields ↩Specifically, without the special-case,
enum Foo { A(Aligned2Never), B(Aligned2Never) }got a size-0 layout with ani16tag field, which I assume would cause problems. ↩